Skip to content

feat: [DevOps] Update sap-rpt specification#792

Merged
rpanackal merged 4 commits intomainfrom
spec-update/sap-rpt/main
Apr 15, 2026
Merged

feat: [DevOps] Update sap-rpt specification#792
rpanackal merged 4 commits intomainfrom
spec-update/sap-rpt/main

Conversation

@bot-sdk-js
Copy link
Copy Markdown
Collaborator

Context

Update sap-rpt specification file based on main.

This PR was created automatically by the spec-update workflow.
You can commit on top of this branch, but as long as this PR is open the action can't be re-run.

  • Compilation outcome: failure
  • Test run outcome: skipped

Before merging, make sure to update tests and release notes, if necessary.

Definition of Done

  • Unit tests cover new classes
  • Release notes updated

newtork
newtork previously approved these changes Mar 23, 2026
Copy link
Copy Markdown
Contributor

@newtork newtork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming Content-Encoding=gzip is still supported.

@Jonas-Isr
Copy link
Copy Markdown
Member

Some background on the failing PR:

Comment on lines -22 to -33
"parameters": [
{
"name": "Content-Encoding",
"in": "header",
"description": "Content encoding of the request body. Use 'gzip' for gzip-compressed payloads.",
"required": false,
"schema": {
"type": "string",
"enum": ["gzip"]
}
}
],
Copy link
Copy Markdown
Member

@rpanackal rpanackal Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, we rely on the manual addition to spec for gzip encoding support. We have drafted an alternate (programatic) solution for passing custom headers in the PR below.

Once, the above PR has been merged (done) and Cloud SDK 5.28.0 has been released, we would need to do the following

  1. Remove the manual addition to spec and regenerate
  2. Set a default header Content-Encoding: gzip for predict() calls using withDefaultHeader() api

This PR doesn't need to be merged afterwards if there are no other changes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The manual changes has been removed and fix applied

Copy link
Copy Markdown
Contributor

@CharlesDuboisSAP CharlesDuboisSAP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM

@Nonnull
public PredictResponsePayload tableCompletion(@Nonnull final PredictRequestPayload requestBody) {
return api.predict(requestBody, "gzip");
return api.withDefaultHeaders(Map.of("Content-Encoding", "gzip")).predict(requestBody);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you store the api.withDefaultHeaders(Map.of("Content-Encoding", "gzip")) alongside the api so we don't recreate objects at every request

@rpanackal rpanackal merged commit 484807b into main Apr 15, 2026
6 checks passed
@rpanackal rpanackal deleted the spec-update/sap-rpt/main branch April 15, 2026 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants